-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: modify Jira tool for API key support #396
Conversation
The Jira API is different for OAuth versus API keys. This change modifies the Jira tool so that it works with API keys and OAuth. Signed-off-by: Donnie Adams <[email protected]>
'Accept': 'application/json', | ||
}, | ||
}) | ||
|
||
let result = null | ||
switch (command) { | ||
case "listJiraSites": | ||
if (process.env.ATLASSIAN_SITE_URL) { | ||
console.log("listJiraSites is not needed when ATLASSIAN_SITE_URL is set, so you can continue without needing the site_id argument") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we just return the first component of the subdomain here instead? The side_id argument is ignored when a PAT credential is used, so it won't matter if it's not a real Jira ID, and we won't need to add the additional line context (and hope the LLM will respect it).
IMO, the only thing that could interfere with that is if the LLM has info about Jira site IDs in its training set and gets confused by the dummy ID. Although, I think it's pretty unlikely given how explicit our tool definitions and context is wrt site_id usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that provide some value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just that it depends less on the LLM doing the right thing based on the context. I've been bitten by that before, especially when a bunch of tools are added to an agent and the context grows large.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this out and it works well for OAuth and the PAT, so I won't block this PR on my earlier suggestion, but I think it's worth considering as an alternative if folks share the same apprehension about the reliability of context-based solutions.
The Jira API is different for OAuth versus API keys. This change modifies the Jira tool so that it works with API keys and OAuth.
Issue: obot-platform/obot#1555